-
-
Notifications
You must be signed in to change notification settings - Fork 32.8k
gh-137658: Fix dataclass order method behaviors to align with the equality semantics #137497
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
Don't use a PR number as the issue prefix, things get confused. Please can you create an issue for this PR, and add a NEWS entry? Please can you use a A |
We need to be very careful that we're not changing semantics with this. We've had that problem before with comparisons (although of course now I can't find the issue). I don't think we added tests for this, because we didn't find the problem until after we'd released the code. |
@ericvsmith so we need set of tests that pass prior to this change? Potentially relevant "semantics" you mention may be shown in the tests for this change in #582? |
I'm specifically thinking of #128294. We don't want to do something like that again. I haven't looked at this change closely, I just want to be extra careful. |
@ericvsmith Thanks for that info, I looked into the situation a bit and it seems that since the behavior of 3.13 for I will file an issue for this
|
@AA-Turner thanks, added those details except the performance part. Rebranded the PR as a fix now that the linked issue is related to aligning behavior semantics with #104904 @ericvsmith I added tests to show the behavior and how it aligns with the 3.13+ equality methods. Some of the assertions would fail on |
Similar to #104904
This can speed up ordering/comparison operations by ~2x by making the comparison similar to existing
__eq__
performance.Benchmark timings for classes with 5 integer fields: